feat(rollout): vLLM prefill/decode (PD) disaggregation via static vllm-router - #166
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for Prefill/Decode (PD) disaggregation in the vLLM engine by launching prefill and decode engines with the NIXL KV connector and configuring unique side-channel ports. It also adds a static PD router startup flow in the rollout module that collects engine URLs before launching the router. The review feedback suggests replacing a process-alive assertion with an explicit conditional check to avoid issues when assertions are disabled in production, and optimizing the sequential ray.get calls inside a loop by batching them into a single parallelized call.
| process.daemon = True | ||
| process.start() | ||
| time.sleep(3) | ||
| assert process.is_alive() |
Contributor
There was a problem hiding this comment.
Using assert for production control flow or process health checks is a bad practice because assertions can be disabled globally in Python when run with the -O (optimize) flag. If assertions are disabled, assert process.is_alive() will be completely ignored, and if the process failed to start or crashed immediately, the code will proceed silently. Use an explicit conditional check and raise a RuntimeError instead.
Suggested change
| assert process.is_alive() | |
| if not process.is_alive(): | |
| raise RuntimeError("Failed to start vLLM-router process.") |
Comment on lines
+1146
to
+1161
| if use_static_pd_router: | ||
| prefill_urls: list[tuple] = [] | ||
| decode_urls: list[str] = [] | ||
| for g in server_groups: | ||
| for e in g.engines: | ||
| if e is None: | ||
| continue | ||
| if g.worker_type == "prefill": | ||
| url, bport = ray.get([e.get_url.remote(), e.get_pd_bootstrap_port.remote()]) | ||
| if url: | ||
| prefill_urls.append((url, bport)) | ||
| elif g.worker_type == "decode": | ||
| url = ray.get(e.get_url.remote()) | ||
| if url: | ||
| decode_urls.append(url) | ||
| _launch_static_pd_router(args, router_ip, router_port, prom_port, prefill_urls, decode_urls) |
Contributor
There was a problem hiding this comment.
Gathering Ray ObjectRefs inside a nested loop and calling ray.get sequentially on each iteration is a performance bottleneck. It forces sequential round-trips to the Ray actors, blocking the main thread. Instead, you should collect all remote calls into a list and resolve them in a single parallelized ray.get call.
if use_static_pd_router:
prefill_engines = []
decode_engines = []
for g in server_groups:
for e in g.engines:
if e is None:
continue
if g.worker_type == "prefill":
prefill_engines.append(e)
elif g.worker_type == "decode":
decode_engines.append(e)
prefill_refs = []
for e in prefill_engines:
prefill_refs.extend([e.get_url.remote(), e.get_pd_bootstrap_port.remote()])
decode_refs = [e.get_url.remote() for e in decode_engines]
all_results = ray.get(prefill_refs + decode_refs)
prefill_results = all_results[:len(prefill_refs)]
decode_results = all_results[len(prefill_refs):]
prefill_urls: list[tuple] = []
for i in range(0, len(prefill_results), 2):
url = prefill_results[i]
bport = prefill_results[i+1]
if url:
prefill_urls.append((url, bport))
decode_urls: list[str] = [url for url in decode_results if url]
_launch_static_pd_router(args, router_ip, router_port, prom_port, prefill_urls, decode_urls)
…m-router Add the PD (prefill/decode) split on top of the colocate DP+EP rollout: - vllm_engine.py: for prefill/decode rollout groups (node_rank 0), pin the NIXL side-channel host/port to the orchestrator-allocated bootstrap port (VLLM_NIXL_SIDE_CHANNEL_HOST/PORT); plumb disaggregation_bootstrap_port through _compute_server_args and persist it on the actor. - rollout.py: _start_router gains a static-PD path (bind + static prefill_urls/decode_urls); start_rollout_servers reserves the router endpoint, starts engines, collects per-worker_type URLs, then launches the vllm-router in PD mode. Engines do not self-register. NIXL pull mode carries the side-channel coords in the prefill response (kv_transfer_params), so prefill_urls use (url, None) -- the per-URL bootstrap port is Mooncake-only and not advertised to the router. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aoshen02
force-pushed
the
feat/vllm-pd-disaggregation
branch
from
June 6, 2026 15:29
ef58226 to
21026bf
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
…rgs) Router.disable_health_check was added speculatively but does not exist in the installed vllm_router version. Router already has disable_circuit_breaker which handles the transient-RDMA concern. Remove the invalid kwarg. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
We’d better test it e2e. |
Collaborator
Author
Cross-Node PD Validation Results (H200 ×2)Setup
Runs
Eval Before Train (iteration 0)
Steady-State Performance (iteration 1, no eval overhead)
TP=2 is the sweet spot — faster weight sync offsets the slightly slower per-engine throughput. Issues Encountered (all environment-level, no vime code bugs)
ConclusionCross-node PD disaggregation via NIXL works correctly on H200 ×2. No new code bugs found. The |
momo609
pushed a commit
that referenced
this pull request
Jun 10, 2026
…m-router (#166) * feat(rollout): vLLM prefill/decode (PD) disaggregation via static vllm-router Add the PD (prefill/decode) split on top of the colocate DP+EP rollout: - vllm_engine.py: for prefill/decode rollout groups (node_rank 0), pin the NIXL side-channel host/port to the orchestrator-allocated bootstrap port (VLLM_NIXL_SIDE_CHANNEL_HOST/PORT); plumb disaggregation_bootstrap_port through _compute_server_args and persist it on the actor. - rollout.py: _start_router gains a static-PD path (bind + static prefill_urls/decode_urls); start_rollout_servers reserves the router endpoint, starts engines, collects per-worker_type URLs, then launches the vllm-router in PD mode. Engines do not self-register. NIXL pull mode carries the side-channel coords in the prefill response (kv_transfer_params), so prefill_urls use (url, None) -- the per-URL bootstrap port is Mooncake-only and not advertised to the router. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Update rollout.py Signed-off-by: aoshen02 <aoshen@inferact.ai> * Update rollout.py Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix(rollout): remove disable_health_check (not in vllm_router RouterArgs) Router.disable_health_check was added speculatively but does not exist in the installed vllm_router version. Router already has disable_circuit_breaker which handles the transient-RDMA concern. Remove the invalid kwarg. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Carves the PD (prefill/decode) disaggregation half out of #108 so that PR can stay DP+EP-only. On top of the colocate DP+EP rollout, this adds the prefill/decode split via the production vllm-router in static mode:
vllm_engine.py— for a rollout group whoseworker_typeisprefill/decode(node_rank 0), pin the NIXL side-channelVLLM_NIXL_SIDE_CHANNEL_HOST/PORTto the orchestrator-allocateddisaggregation_bootstrap_port, plumbed through_compute_server_argsand persisted on the actor. The KV connector (--kv-transfer-config) itself is supplied by the operator via normal vLLM arg passthrough.rollout.py—_start_routergains a static-PD path (bind+ staticprefill_urls/decode_urls, health check disabled).start_rollout_serversreserves the router endpoint, starts engines (which do not self-register), collects per-worker_typeURLs, then launches the router in PD mode.NIXL pull mode → no per-URL bootstrap port
vime uses vLLM's NIXL pull connector: the prefill engine returns the side-channel coords (
remote_host/remote_port/remote_block_ids) to the decode side at request time via the response'skv_transfer_params. The router never consumes a per-prefill-URL bootstrap port (that path is Mooncake-only). Soprefill_urlsare(url, None)tuples — matching SkyRL's static-PD reference andvllm_router._parse_prefill_urls(which explicitly acceptsnone).Relationship to #108
build_vllm_cmd_and_env, so they compose without conflict.Test
py_compileclean on both files.(url, None)tuple shape. One pre-existing latent crash surfaced in the refactored_start_router(the router-reuse early-return returned a 2-tuple while callers unpack 3) and is fixed here (returns(ip, port, None)).🤖 Generated with Claude Code